Skip to content

Feat/sw 61 폴더 CRUD 로직 마무리 작업#35

Open
ekdh0858 wants to merge 8 commits intodevfrom
feat/SW-61
Open

Feat/sw 61 폴더 CRUD 로직 마무리 작업#35
ekdh0858 wants to merge 8 commits intodevfrom
feat/SW-61

Conversation

@ekdh0858
Copy link
Contributor

@ekdh0858 ekdh0858 commented Mar 8, 2026

💡 이슈

resolve {#SW-61}

🤩 개요

PR의 개요를 적어주세요.
폴더 CRUD 기능의 비지니스 로직 및 검증 로직 보완
규칙에 맞는 예외처리

🧑‍💻 작업 사항

https://searchweb.atlassian.net/wiki/x/A4BJAQ

📖 참고 사항

공유할 내용, 레퍼런스, 추가로 발생할 것으로 예상되는 이슈, 스크린샷 등을 넣어 주세요.

Summary by CodeRabbit

릴리스 노트

  • New Features

    • 폴더 작업에 사용자 인증 기능 추가
    • 폴더명 및 설명에 대한 유효성 검증 강화 (글자 수 제한)
    • 미인증 사용자 접근 제한 오류 코드 추가
  • Bug Fixes

    • 북마크가 포함된 폴더 삭제 방지
    • 폴더 이동 시 소유권 검증 및 순환 참조 감지
    • 폴더명 중복 검사 개선
  • Refactor

    • 설정 관련 패키지 구조 재정렬

ekdh0858 added 8 commits March 5, 2026 17:21
Exception 사용시 상황에 맞는 메서드를 생성하는 과정을 삭제하고, 대신 에러코드를 Exception을 생성할때 사용해서 나타내는 방식 사용
common : 공통 응답 형식
exception : 예외처리 관련
security : 시큐리티 설정 관련
로 정리함
Security를 통해서 로그인을 한 사람의 Id를 사용할 수 있도록 util클래스 생성.
추후 Security를 통해서 무언가를 자여와야 할 때 이 클래스를 사용
폴더의 조회 관련 메서드에서 필요한 폴더내 활성 북마크들을 전부 불러오는 시능 추가
폴더의 비지니스 로직 마저 완성 자세한건 컨플루언스 문서로 기록
로그인 관련 에러코드 및
SecurityUtils 내용 추가 자세한 내용은 컨플루언스 문서로 작성
@coderabbitai
Copy link

coderabbitai bot commented Mar 8, 2026

전체 요약

이 변경사항은 패키지 구조를 재정리하여 config 하위에 common, exception, security 서브패키지를 신규 생성합니다. SecurityUtils 클래스를 추가해 인증된 사용자의 멤버ID 추출을 중앙화하고, 폴더 컨트롤러와 서비스 계층에서 인증 기반 소유권 검증 로직을 도입합니다. DTO 검증, 폴더 이동 및 삭제 시 순환 참조 방지 및 중복 검사를 강화하며, 새로운 에러 코드(UNAUTHORIZED, FOLDER_FORBIDDEN 등)를 추가합니다.

시퀀스 다이어그램

sequenceDiagram
    actor Client
    participant Controller as MemberFolderController
    participant SecurityUtils
    participant Service as MemberFolderServiceImpl
    participant Dao as MemberFolderJpaDao
    participant BookmarkDao
    
    Client->>Controller: 폴더 작업 요청<br/>(create/update/delete)
    Controller->>SecurityUtils: extractMemberId(Authentication)
    SecurityUtils-->>Controller: loginId (멤버ID)
    
    Controller->>Service: 작업 수행<br/>(loginId 포함)
    
    Service->>Dao: 소유권 검증<br/>(loginId로 폴더 조회)
    Dao-->>Service: 폴더 정보
    
    alt 작업이 delete인 경우
        Service->>BookmarkDao: existsActiveBookmarkInFolder()<br/>(활성 북마크 확인)
        BookmarkDao-->>Service: boolean
        Service->>Dao: 폴더 삭제 가능 여부 확인<br/>(자식 폴더 존재 여부)
        Dao-->>Service: boolean
    else 작업이 move인 경우
        Service->>Dao: 순환 참조 검증<br/>(새로운 부모 폴더)
        Dao-->>Service: 유효성 결과
        Service->>Dao: 중복 폴더명 검사<br/>(새 위치에서)
        Dao-->>Service: 중복 여부
    end
    
    Service->>Dao: 작업 수행<br/>(DB 변경)
    Dao-->>Service: 결과
    Service-->>Controller: 작업 완료
    Controller-->>Client: ApiResponse
Loading

코드 리뷰 난이도

🎯 4 (Complex) | ⏱️ ~45분

🐰 패키지를 정렬하고, 소유권을 지키며,
인증의 벽을 세우니 탄탄해졌네!
순환은 막고, 중복은 검사하고,
북마크도 안전하게 보호하며
폴더의 나라는 더욱 견고해졌다! 🛡️✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive 필수 섹션들이 모두 포함되어 있으나, '개요' 섹션에서 실제 작업 내용이 부분적으로만 작성되었고, '작업 사항'은 외부 링크만 제공되어 구체적인 내용이 부족합니다. PR 설명에 폴더 CRUD 로직 보완의 구체적인 작업 내용, 주요 변경사항, 예외 처리 방식 변경 등을 직접 기술하여 보강해 주세요.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed 제목이 폴더 CRUD 로직 마무리 작업이라는 주요 변경사항을 명확하게 요약하고 있으며, PR의 핵심 목적을 잘 반영하고 있습니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/SW-61

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java (1)

110-118: ⚠️ Potential issue | 🟠 Major

중복 Bean 생성 문제

CustomAuthenticationFailureHandlerCustomAuthenticationSuccessHandler는 이미 @Component로 선언되어 Spring이 자동으로 Bean을 생성합니다. 여기서 new로 추가 인스턴스를 생성하면 중복 Bean이 만들어집니다. 생성자 주입으로 기존 Bean을 사용하거나, 핸들러 클래스에서 @Component를 제거하세요.

♻️ 제안하는 수정 사항 (생성자 주입 방식)
 public class SecurityConfig {

     private final CustomOAuth2UserService customOAuth2UserService;
+    private final CustomAuthenticationSuccessHandler customAuthenticationSuccessHandler;
+    private final CustomAuthenticationFailureHandler customAuthenticationFailureHandler;

-    public SecurityConfig(CustomOAuth2UserService customOAuth2UserService) {
+    public SecurityConfig(CustomOAuth2UserService customOAuth2UserService,
+                          CustomAuthenticationSuccessHandler customAuthenticationSuccessHandler,
+                          CustomAuthenticationFailureHandler customAuthenticationFailureHandler) {
         this.customOAuth2UserService = customOAuth2UserService;
+        this.customAuthenticationSuccessHandler = customAuthenticationSuccessHandler;
+        this.customAuthenticationFailureHandler = customAuthenticationFailureHandler;
     }

그리고 filterChain 메서드에서:

-                        .successHandler(customAuthenticationSuccessHandler())
-                        .failureHandler(customAuthenticationFailureHandler())
+                        .successHandler(customAuthenticationSuccessHandler)
+                        .failureHandler(customAuthenticationFailureHandler)

마지막으로 @Bean 메서드들을 제거하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java` around
lines 110 - 118, SecurityConfig currently defines beans
customAuthenticationFailureHandler() and customAuthenticationSuccessHandler()
which duplicate the existing `@Component-scanned`
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler;
remove these `@Bean` methods and instead inject the existing beans into
SecurityConfig via constructor injection (add constructor parameters
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler) and
use those injected instances inside the filterChain method; alternatively, if
you prefer explicit bean factory methods, remove the `@Component` annotation from
the handler classes so only the `@Bean` factory methods produce instances.
src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java (1)

17-21: ⚠️ Potential issue | 🔴 Critical

오픈 리다이렉트 취약점 위험

redirect 파라미터가 검증 없이 sendRedirect()에 직접 전달되고 있습니다. 공격자가 redirect=https://malicious-site.com 형태로 악용하여 피싱 공격에 사용할 수 있습니다. 내부 경로 또는 허용된 도메인인지 검증이 필요합니다.

🔒 제안하는 수정 사항
 `@Override`
 public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException {
     // 리다이렉트 URL 설정
     String redirectUrl = request.getParameter("redirect");

-    if (redirectUrl != null && !redirectUrl.isEmpty()) {
+    if (redirectUrl != null && !redirectUrl.isEmpty() && isValidRedirectUrl(redirectUrl)) {
         // 리디렉트 파라미터가 있을 경우 그 URL로 리다이렉트
         response.sendRedirect(redirectUrl);
     } else {
         // 없을 경우 기본 페이지로 이동
         response.sendRedirect("/mainList");
     }
 }
+
+private boolean isValidRedirectUrl(String url) {
+    // 상대 경로만 허용하거나, 허용된 도메인 목록과 비교
+    return url.startsWith("/") && !url.startsWith("//");
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java`
around lines 17 - 21, The redirectUrl parameter in
CustomAuthenticationSuccessHandler is used directly in response.sendRedirect,
causing an open-redirect risk; update the onAuthenticationSuccess logic to
validate redirectUrl before redirecting: parse the redirectUrl (variable
redirectUrl) and only allow redirection if it is a safe relative path (starts
with "/" but not "//" or contains protocol) or matches an explicit allowlist of
trusted domains, otherwise fall back to the default target (e.g.,
defaultTargetUrl or savedRequestRedirect). Ensure this validation occurs
immediately before the call to response.sendRedirect and reject/ignore unsafe
values.
src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java (1)

11-11: 🧹 Nitpick | 🔵 Trivial

Enum 상수 네이밍 컨벤션 위반

Tag_NOT_FOUND는 Java enum 네이밍 컨벤션(SCREAMING_SNAKE_CASE)을 따르지 않습니다. 다른 ErrorCode enum들(DUPLICATE_BOOKMARK, FOLDER_NOT_FOUND 등)과 일관성을 위해 TAG_NOT_FOUND로 변경하는 것을 권장합니다.

♻️ 제안하는 수정
-    Tag_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");
+    TAG_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");

참고: TagException.java에서 TagErrorCode.Tag_NOT_FOUND 참조도 함께 수정이 필요합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java` at line 11,
Rename the enum constant Tag_NOT_FOUND to follow SCREAMING_SNAKE_CASE as
TAG_NOT_FOUND in TagErrorCode (change the symbol Tag_NOT_FOUND -> TAG_NOT_FOUND)
and update all references, e.g., in TagException (replace
TagErrorCode.Tag_NOT_FOUND with TagErrorCode.TAG_NOT_FOUND) so naming is
consistent with other constants like DUPLICATE_BOOKMARK and FOLDER_NOT_FOUND.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/web/SearchWeb/config/security/SecurityUtils.java`:
- Around line 25-30: In SecurityUtils (the method that extracts memberId from
the authentication principal), ensure you fail-closed by validating the returned
memberId from CustomUserDetails.getMemberId() and
CustomOAuth2User.getMemberId(): if either is null throw an UNAUTHORIZED response
(e.g. throw ResponseStatusException(HttpStatus.UNAUTHORIZED) or your app's
equivalent) instead of returning null; also handle the fallback branch (when
principal is not one of those types) to throw UNAUTHORIZED so no null memberId
propagates.

In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`:
- Around line 17-27: Change the DTO fields from public to private so external
code cannot mutate them after validation; in the nested request class Create
make parentFolderId, folderName, and description private and rely on the
existing `@Getter` (and default constructor) for Jackson/Bean Validation, and
apply the same change to the other nested request classes in this file (e.g.,
Update/other request classes that currently have public fields).

In `@src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java`:
- Line 13: The enum constant FOLDER_FORBIDDEN in FolderErrorCode currently uses
the incorrect code string "Foo3"; update the enum entry for FOLDER_FORBIDDEN to
use the correct code "F003" so the exposed error code matches the response
contract and client documentation.

In `@src/main/java/com/web/SearchWeb/folder/error/FolderException.java`:
- Line 10: The FolderException constructor currently accepts the generic
ErrorCode which allows non-folder error codes to be passed; change the
constructor signature in class FolderException to accept FolderErrorCode instead
(replace ErrorCode parameter with FolderErrorCode) and update all call sites
that construct new FolderException(...) to pass a FolderErrorCode (adjust
imports/usages as needed) so the API boundary is type-safe; keep the rest of
FolderException behavior unchanged.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`:
- Around line 13-15: Remove the redundant ownerMemberId parameter from the
MemberFolderService API: change the signatures of listRootFolders and
listChildren to accept only Long loginId (and Long parentFolderId for
listChildren), and update all implementations to derive the owner from loginId
instead of using ownerMemberId; also remove or adapt calls to
validateOwner(loginId, ownerMemberId) so they validate ownership using loginId
alone (e.g., validateOwner(loginId) or inline owner resolution) and adjust any
controllers/routes to stop passing ownerMemberId.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`:
- Around line 220-235: validateNoCycle currently walks parent links and issues a
DB call per level via memberFolderJpaRepository.findById, which can be slow for
deep hierarchies; replace or supplement this with a single recursive query (CTE)
in the repository to fetch the ancestor chain (or perform a direct cycle check)
and use that result to detect folderId in the ancestors, updating
validateNoCycle to call the new repository method instead of looping; reference
validateNoCycle, memberFolderJpaRepository.findById, and the
FolderException/FOLDER_NOT_FOUND and INVALID_FOLDER_MOVE error codes when adding
the repository method and adjusting the service logic.
- Around line 80-91: normalizeDescription currently trims and null-checks but
lacks the max-length validation present in normalizeFolderName; add a length
check to ensure the returned description does not exceed the MemberFolder
`@Column`(length = 500) constraint. Modify normalizeDescription to trim, return
null for empty, then if length > 500 either truncate to 500 or throw an
IllegalArgumentException (match the approach used in normalizeFolderName), and
prefer introducing/using a named constant (e.g., MAX_DESCRIPTION_LENGTH = 500)
to make the limit explicit.
- Around line 68-78: The service validation in normalizeFolderName enforces a
50-character limit that conflicts with MemberFolder.folderName which is
annotated `@Column`(length = 100); fix by making the constraints consistent:
either update normalizeFolderName to allow up to 100 characters (replace the 50
check) or change the MemberFolder.folderName column length to 50 and update any
DB migrations; ensure FolderException uses FolderErrorCode.INVALID_FOLDER_NAME
as before and, if the shorter 50 limit is intentional, add a clear comment or
Javadoc near MemberFolder.folderName and normalizeFolderName documenting the
business rule.

---

Outside diff comments:
In
`@src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java`:
- Around line 17-21: The redirectUrl parameter in
CustomAuthenticationSuccessHandler is used directly in response.sendRedirect,
causing an open-redirect risk; update the onAuthenticationSuccess logic to
validate redirectUrl before redirecting: parse the redirectUrl (variable
redirectUrl) and only allow redirection if it is a safe relative path (starts
with "/" but not "//" or contains protocol) or matches an explicit allowlist of
trusted domains, otherwise fall back to the default target (e.g.,
defaultTargetUrl or savedRequestRedirect). Ensure this validation occurs
immediately before the call to response.sendRedirect and reject/ignore unsafe
values.

In `@src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java`:
- Around line 110-118: SecurityConfig currently defines beans
customAuthenticationFailureHandler() and customAuthenticationSuccessHandler()
which duplicate the existing `@Component-scanned`
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler;
remove these `@Bean` methods and instead inject the existing beans into
SecurityConfig via constructor injection (add constructor parameters
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler) and
use those injected instances inside the filterChain method; alternatively, if
you prefer explicit bean factory methods, remove the `@Component` annotation from
the handler classes so only the `@Bean` factory methods produce instances.

In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java`:
- Line 11: Rename the enum constant Tag_NOT_FOUND to follow SCREAMING_SNAKE_CASE
as TAG_NOT_FOUND in TagErrorCode (change the symbol Tag_NOT_FOUND ->
TAG_NOT_FOUND) and update all references, e.g., in TagException (replace
TagErrorCode.Tag_NOT_FOUND with TagErrorCode.TAG_NOT_FOUND) so naming is
consistent with other constants like DUPLICATE_BOOKMARK and FOLDER_NOT_FOUND.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f51df56-6d63-40ce-b49a-be78265abe37

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad5207 and b9f26d6.

📒 Files selected for processing (26)
  • src/main/java/com/web/SearchWeb/bookmark/controller/BookmarkApiController.java
  • src/main/java/com/web/SearchWeb/bookmark/dao/BookmarkDao.java
  • src/main/java/com/web/SearchWeb/bookmark/dao/MybatisBookmarkDao.java
  • src/main/java/com/web/SearchWeb/bookmark/error/BookmarkErrorCode.java
  • src/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.java
  • src/main/java/com/web/SearchWeb/config/common/ApiResponse.java
  • src/main/java/com/web/SearchWeb/config/exception/BusinessException.java
  • src/main/java/com/web/SearchWeb/config/exception/CommonErrorCode.java
  • src/main/java/com/web/SearchWeb/config/exception/ErrorCode.java
  • src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationFailureHandler.java
  • src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java
  • src/main/java/com/web/SearchWeb/config/security/GlobalExceptionHandler.java
  • src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java
  • src/main/java/com/web/SearchWeb/config/security/SecurityUtils.java
  • src/main/java/com/web/SearchWeb/folder/controller/MemberFolderController.java
  • src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java
  • src/main/java/com/web/SearchWeb/folder/dao/MemberFolderJpaDao.java
  • src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java
  • src/main/java/com/web/SearchWeb/folder/error/FolderException.java
  • src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java
  • src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java
  • src/main/java/com/web/SearchWeb/mypage/controller/MyPageController.java
  • src/main/java/com/web/SearchWeb/tag/controller/MemberTagController.java
  • src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java
  • src/main/java/com/web/SearchWeb/tag/error/TagException.java
  • src/main/resources/mapper/bookmark-mapper.xml

Comment on lines +25 to +30
if (principal instanceof CustomUserDetails userDetails) {
return userDetails.getMemberId();
}

if (principal instanceof CustomOAuth2User oauth2User) {
return oauth2User.getMemberId();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

memberId가 null인 경우도 여기서 차단하세요.

CustomUserDetails#getMemberId()CustomOAuth2User#getMemberId()가 값을 그대로 반환하므로, 인증 객체가 만들어졌더라도 memberId가 null이면 이후 권한 체크가 500으로 깨질 수 있습니다. 이 메서드에서 fail-closed로 UNAUTHORIZED를 던지는 편이 안전합니다.

🔒 제안 수정
         if (principal instanceof CustomUserDetails userDetails) {
-            return userDetails.getMemberId();
+            Long memberId = userDetails.getMemberId();
+            if (memberId != null) {
+                return memberId;
+            }
+            throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
         }

         if (principal instanceof CustomOAuth2User oauth2User) {
-            return oauth2User.getMemberId();
+            Long memberId = oauth2User.getMemberId();
+            if (memberId != null) {
+                return memberId;
+            }
+            throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (principal instanceof CustomUserDetails userDetails) {
return userDetails.getMemberId();
}
if (principal instanceof CustomOAuth2User oauth2User) {
return oauth2User.getMemberId();
if (principal instanceof CustomUserDetails userDetails) {
Long memberId = userDetails.getMemberId();
if (memberId != null) {
return memberId;
}
throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
}
if (principal instanceof CustomOAuth2User oauth2User) {
Long memberId = oauth2User.getMemberId();
if (memberId != null) {
return memberId;
}
throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/config/security/SecurityUtils.java` around
lines 25 - 30, In SecurityUtils (the method that extracts memberId from the
authentication principal), ensure you fail-closed by validating the returned
memberId from CustomUserDetails.getMemberId() and
CustomOAuth2User.getMemberId(): if either is null throw an UNAUTHORIZED response
(e.g. throw ResponseStatusException(HttpStatus.UNAUTHORIZED) or your app's
equivalent) instead of returning null; also handle the fallback branch (when
principal is not one of those types) to throw UNAUTHORIZED so no null memberId
propagates.

Comment on lines 17 to 27
public static class Create {
public Long ownerMemberId;
public Long parentFolderId; // null이면 루트
// null이면 루트 폴더
@Positive(message = "parentFolderId는 양수여야 합니다.")
public Long parentFolderId;

@NotBlank(message = "folderName은 비어 있을 수 없습니다.")
@Size(max = 50, message = "folderName은 최대 50자까지 가능합니다.")
public String folderName;

@Size(max = 200, message = "description은 최대 200자까지 가능합니다.")
public String description;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

요청 DTO 필드는 private로 두세요.

@Getter를 붙였는데 필드가 여전히 public이라 검증/바인딩 이후에도 외부 코드가 값을 직접 바꿀 수 있습니다. Jackson + Bean Validation 조합에서는 기본 생성자와 private 필드만으로 충분합니다.

♻️ 제안 수정
     public static class Create {
         // null이면 루트 폴더
         `@Positive`(message = "parentFolderId는 양수여야 합니다.")
-        public Long parentFolderId;
+        private Long parentFolderId;

         `@NotBlank`(message = "folderName은 비어 있을 수 없습니다.")
         `@Size`(max = 50, message = "folderName은 최대 50자까지 가능합니다.")
-        public String folderName;
+        private String folderName;

         `@Size`(max = 200, message = "description은 최대 200자까지 가능합니다.")
-        public String description;
+        private String description;
     }
@@
     public static class Update {
         `@Size`(max = 50, message = "folderName은 최대 50자까지 가능합니다.")
-        public String folderName;
+        private String folderName;

         `@Size`(max = 200, message = "description은 최대 200자까지 가능합니다.")
-        public String description;
+        private String description;
     }
@@
     public static class Move {
         // null이면 루트로 이동
         `@Positive`(message = "newParentFolderId는 양수여야 합니다.")
-        public Long newParentFolderId;
+        private Long newParentFolderId;
     }

Also applies to: 35-40, 48-51

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`
around lines 17 - 27, Change the DTO fields from public to private so external
code cannot mutate them after validation; in the nested request class Create
make parentFolderId, folderName, and description private and rely on the
existing `@Getter` (and default constructor) for Jackson/Bean Validation, and
apply the same change to the other nested request classes in this file (e.g.,
Update/other request classes that currently have public fields).

FOLDER_NOT_FOUND(HttpStatus.NOT_FOUND, "F001", "폴더를 찾을 수 없습니다."),
DUPLICATE_FOLDER_NAME(HttpStatus.BAD_REQUEST, "F002", "이미 존재하는 폴더명입니다.");
DUPLICATE_FOLDER_NAME(HttpStatus.BAD_REQUEST, "F002", "이미 존재하는 폴더명입니다."),
FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

에러 코드 값 오타를 수정하세요.

FOLDER_FORBIDDEN의 코드가 Foo3로 들어가 있습니다. 이 값이 응답 계약에 노출되면 클라이언트 분기와 문서가 모두 어긋나므로 F003로 바로잡아야 합니다.

🐛 제안 수정
-    FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ),
+    FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN, "F003", "접근이 제한된 폴더입니다."),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ),
FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN, "F003", "접근이 제한된 폴더입니다."),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java` at line
13, The enum constant FOLDER_FORBIDDEN in FolderErrorCode currently uses the
incorrect code string "Foo3"; update the enum entry for FOLDER_FORBIDDEN to use
the correct code "F003" so the exposed error code matches the response contract
and client documentation.

public class FolderException extends BusinessException {

private FolderException(ErrorCode errorCode) {
public FolderException(ErrorCode errorCode) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

FolderException 생성자는 FolderErrorCode로 좁히는 편이 안전합니다.

지금 시그니처면 폴더 도메인 예외에 다른 도메인의 ErrorCode를 실수로 태워도 컴파일이 통과합니다. 현재 사용 패턴도 전부 FolderErrorCode라서 타입을 좁혀 두는 쪽이 API 경계를 더 명확하게 만듭니다.

♻️ 제안 수정
-import com.web.SearchWeb.config.exception.ErrorCode;
 import lombok.Getter;

 `@Getter`
 public class FolderException extends BusinessException {

-    public FolderException(ErrorCode errorCode) {
+    public FolderException(FolderErrorCode errorCode) {
         super(errorCode);
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/error/FolderException.java` at line
10, The FolderException constructor currently accepts the generic ErrorCode
which allows non-folder error codes to be passed; change the constructor
signature in class FolderException to accept FolderErrorCode instead (replace
ErrorCode parameter with FolderErrorCode) and update all call sites that
construct new FolderException(...) to pass a FolderErrorCode (adjust
imports/usages as needed) so the API boundary is type-safe; keep the rest of
FolderException behavior unchanged.

Comment on lines +13 to +15
List<MemberFolder> listRootFolders(Long loginId, Long ownerMemberId);

List<MemberFolder> listChildren(Long ownerMemberId, Long parentFolderId);
List<MemberFolder> listChildren(Long loginId, Long ownerMemberId, Long parentFolderId);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd 'MemberFolder(Controller|ServiceImpl)\.java$' src/main/java -x sh -c '
  echo "== $1 ==";
  rg -n -C3 "listRootFolders|listChildren|ownerMemberId|loginId" "$1";
' sh {}

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 8312


조회 API의 ownerMemberId 파라미터는 이미 검증이 적용되어 있습니다.

listRootFolders/listChildrenownerMemberId를 받지만, validateOwner(loginId, ownerMemberId) 호출 시 ownerMemberIdloginId가 일치하는지 검증합니다. 일치하지 않으면 즉시 FolderException(FOLDER_FORBIDDEN)을 발생시키므로, 컨트롤러가 요청값을 그대로 넘겨도 인증 경계는 보호됩니다.

다만 API 설계상 ownerMemberId 파라미터를 제거하고 loginId에서만 소유자를 취득하면 의도가 더 명확해질 수 있습니다. 예를 들어 /root/children/{parentFolderId} 엔드포인트로 단순화하면 URL 경로에서 명시적 검증 로직이 필요 없게 됩니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`
around lines 13 - 15, Remove the redundant ownerMemberId parameter from the
MemberFolderService API: change the signatures of listRootFolders and
listChildren to accept only Long loginId (and Long parentFolderId for
listChildren), and update all implementations to derive the owner from loginId
instead of using ownerMemberId; also remove or adapt calls to
validateOwner(loginId, ownerMemberId) so they validate ownership using loginId
alone (e.g., validateOwner(loginId) or inline owner resolution) and adjust any
controllers/routes to stop passing ownerMemberId.

Comment on lines +68 to +78
private String normalizeFolderName(String folderName) {
if (folderName == null) {
throw new FolderException(FolderErrorCode.INVALID_FOLDER_NAME);
}

String normalizedFolderName = folderName.trim();
if (normalizedFolderName.isEmpty() || normalizedFolderName.length() > 50) {
throw new FolderException(FolderErrorCode.INVALID_FOLDER_NAME);
}
return normalizedFolderName;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check MemberFolder entity for folderName column length definition
ast-grep --pattern $'@Column($$$length = $_$$$)
private String folderName;'

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 54


🏁 Script executed:

fd -name "MemberFolder.java" -type f

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 302


🏁 Script executed:

fd "MemberFolder.java"

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 135


🏁 Script executed:

cat -n "src/main/java/com/web/SearchWeb/folder/domain/MemberFolder.java"

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 1846


서비스 검증 로직과 엔티티 컬럼 길이 불일치

MemberFolder 엔티티의 folderName 필드가 @Column(length = 100)으로 정의되어 있으나, normalizeFolderName 메서드에서는 50자 제한을 적용하고 있습니다. 데이터베이스 제약과 서비스 로직 간 일관성을 맞추거나, 차이가 의도된 비즈니스 규칙이라면 명시적으로 문서화하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 68 - 78, The service validation in normalizeFolderName enforces a
50-character limit that conflicts with MemberFolder.folderName which is
annotated `@Column`(length = 100); fix by making the constraints consistent:
either update normalizeFolderName to allow up to 100 characters (replace the 50
check) or change the MemberFolder.folderName column length to 50 and update any
DB migrations; ensure FolderException uses FolderErrorCode.INVALID_FOLDER_NAME
as before and, if the shorter 50 limit is intentional, add a clear comment or
Javadoc near MemberFolder.folderName and normalizeFolderName documenting the
business rule.

Comment on lines +80 to +91
private String normalizeDescription(String description) {
if (description == null) {
return null;
}

String normalizedDescription = description.trim();
if (normalizedDescription.isEmpty()) {
return null;
}

return normalizedDescription;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

설명(description) 최대 길이 검증 누락

normalizeFolderName은 최대 길이를 검증하지만, normalizeDescription은 길이 검증이 없습니다. MemberFolder 엔티티에서 description은 @Column(length = 500)으로 정의되어 있으므로, 500자를 초과하면 DB 제약 조건 위반으로 런타임 예외가 발생할 수 있습니다.

🛡️ 길이 검증 추가 제안
 private String normalizeDescription(String description) {
     if (description == null) {
         return null;
     }

     String normalizedDescription = description.trim();
     if (normalizedDescription.isEmpty()) {
         return null;
     }

+    if (normalizedDescription.length() > 500) {
+        throw new FolderException(FolderErrorCode.INVALID_DESCRIPTION);
+    }
+
     return normalizedDescription;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 80 - 91, normalizeDescription currently trims and null-checks but
lacks the max-length validation present in normalizeFolderName; add a length
check to ensure the returned description does not exceed the MemberFolder
`@Column`(length = 500) constraint. Modify normalizeDescription to trim, return
null for empty, then if length > 500 either truncate to 500 or throw an
IllegalArgumentException (match the approach used in normalizeFolderName), and
prefer introducing/using a named constant (e.g., MAX_DESCRIPTION_LENGTH = 500)
to make the limit explicit.

Comment on lines +220 to 235
private void validateNoCycle(Long folderId, MemberFolder parentFolder) {
MemberFolder current = parentFolder;
while (current != null) {
if (current.getMemberFolderId().equals(folderId)) {
throw new FolderException(FolderErrorCode.INVALID_FOLDER_MOVE);
}

Long nextParentId = current.getParentFolderId();
if (nextParentId == null) {
return;
}

current = memberFolderJpaRepository.findById(nextParentId)
.orElseThrow(() -> new FolderException(FolderErrorCode.FOLDER_NOT_FOUND));
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

순환 참조 검증 로직 적절함, 성능 고려 권장

validateNoCycle의 로직이 정확합니다. 다만, 깊은 폴더 계층 구조에서는 여러 번의 DB 조회가 발생할 수 있습니다. 현재 구현은 일반적인 사용 사례에서 충분하지만, 폴더 계층이 깊어질 경우 재귀 CTE 쿼리로 최적화를 고려할 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 220 - 235, validateNoCycle currently walks parent links and issues
a DB call per level via memberFolderJpaRepository.findById, which can be slow
for deep hierarchies; replace or supplement this with a single recursive query
(CTE) in the repository to fetch the ancestor chain (or perform a direct cycle
check) and use that result to detect folderId in the ancestors, updating
validateNoCycle to call the new repository method instead of looping; reference
validateNoCycle, memberFolderJpaRepository.findById, and the
FolderException/FOLDER_NOT_FOUND and INVALID_FOLDER_MOVE error codes when adding
the repository method and adjusting the service logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant